Imported global functions #167
Conversation
…s high-throughput component
|
Ran the benchmars - this makes ~10% impact - wasn't expecting that O_o |
Ow it could get even faster if we would jump to PHP 7.0+ (which I personally would love to see in event loop 1.1 (hint hint @clue @jsor)). And use
Yeah that's a thing :(. As a said 1.1? |
|
Ah, interesting, tests fail because of ancient PHPUnit versions. Will have to use a real timer instead of a mock. |
…e being used in CI
Ocramius
force-pushed
the
feature/imported-global-functions
branch
from
d83c940
to
b048bd7
Compare
Ocramius
force-pushed
the
feature/imported-global-functions
branch
from
5fded13
to
88d35a6
Compare
There was a problem hiding this comment.
@Ocramius Thank you very much for filing this PR, the changes LGTM and I can confirm some noticeable performance improvements here 👍 🎉
It looks like this PR currently addresses two things, may I ask you to squash this into two commits? 
| $timers = new Timers(); | ||
|
|
||
| $timer1 = new Timer(0.1, function () {}); | ||
| $timer2 = new Timer(0.1, function () {}); |
There was a problem hiding this comment.
LGTM as-is, but for the reference and because I'm sure you're going to appreciate this: Legacy PHPUnit does not support $this->createMock('React\EventLoop\TimerInterface'), but this can easily be worked around with $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock(). 😉
There was a problem hiding this comment.
Yeah, I avoided the mock builder and went with the real instance. Would've used an anonymous class if this was 7.0+
There was a problem hiding this comment.
LGTM, but can you as requested by @clue squash it in two logical commits?
There are 4 atomic commits that are not supposed to be squashed |
Makes sense 👍 |
Debatable - but I don't really like the idea of discussing this, given this this PR includes some very real improvements, thank you! |
This keeps me awake for now. Silly patch, still kinda relevant for this low-level component
Note that I wanted to use
doctrine/coding-standardon it, but the package seems to still support PHP 5.3 (!!!!!!!!!!!!!!!!!!).